Skip to content

Conversation

@spike-rabbit
Copy link
Member

@spike-rabbit spike-rabbit commented Nov 11, 2025

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

ghostLoadingIndicator is a decorator input.

What is the new behavior?
ghostLoadingIndicator is a signal.

Does this PR introduce a breaking change? (check one with "x")

  • Yes, but covered in another commit
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

Using effects to handle side effects is supposed to be a temporary solution. We must convert most fields to signals, before we can use computed instead.

@spike-rabbit spike-rabbit marked this pull request as ready for review November 11, 2025 08:23
@spike-rabbit spike-rabbit requested a review from a team as a code owner November 11, 2025 08:23
@fh1ch fh1ch requested a review from Copilot November 11, 2025 13:00
Copilot finished reviewing on behalf of fh1ch November 11, 2025 13:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the ghostLoadingIndicator property from a traditional decorator-based @Input with getter/setter to a modern signal-based input. The conversion aligns with Angular's signal-based reactivity model and removes the deprecated approach of using setter logic for side effects.

  • Converted ghostLoadingIndicator from @Input() with getter/setter to input() signal
  • Moved side effect logic from the setter into an effect() block in the constructor
  • Updated template binding to call ghostLoadingIndicator() as a signal function
  • Removed the private _ghostLoadingIndicator backing field

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
projects/ngx-datatable/src/lib/components/datatable.component.ts Converts ghostLoadingIndicator to signal input, moves side effect logic to effect(), updates signal call, and removes backing field
projects/ngx-datatable/src/lib/components/datatable.component.html Updates template binding to call ghostLoadingIndicator() as a signal function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fh1ch fh1ch added the enhancement New feature or request label Nov 16, 2025
Copy link
Member

@fh1ch fh1ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spike-rabbit flawless work here, thanks a bunch 🙇

LGTM 👍

@fh1ch fh1ch force-pushed the refactor/ghost-loading-indicator branch from d407473 to a6cfaf1 Compare November 16, 2025 11:34
@fh1ch fh1ch enabled auto-merge (rebase) November 16, 2025 11:34
@fh1ch fh1ch merged commit 467adba into main Nov 16, 2025
3 checks passed
@fh1ch fh1ch deleted the refactor/ghost-loading-indicator branch November 16, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants